Skip to content

fix Unsafe shell command constructed from library input shell-quote() OS Command Injection#14172

Merged
sobolk merged 1 commit intoaws-amplify:devfrom
odaysec:patch-1
Apr 21, 2025
Merged

fix Unsafe shell command constructed from library input shell-quote() OS Command Injection#14172
sobolk merged 1 commit intoaws-amplify:devfrom
odaysec:patch-1

Conversation

@odaysec
Copy link
Copy Markdown
Contributor

@odaysec odaysec commented Apr 12, 2025

const lambdaExecutablePath = path.join(lambdaExecutableDir, MAIN_BINARY);

To fix the problem, we should avoid using execa.command with a dynamically constructed command string. Instead, we can use execa with an array of arguments to safely pass the executable and its arguments without shell interpretation. This approach ensures that the input is not interpreted by the shell, preventing command injection.

  1. Replace the use of execa.command with execa and pass the executable and arguments as an array.
  2. Ensure that the lambdaExecutablePath and lambdaExecutableDir are passed as separate arguments to avoid shell interpretation.

Dynamically constructing a shell command with inputs from exported functions may inadvertently change the meaning of the shell command. Clients using the exported function may use inputs containing characters that the shell interprets in a special way, for instance quotes and spaces. This can result in the shell command misbehaving, or even allowing a malicious user to execute arbitrary commands on the system.

POC

The following shows a dynamically constructed shell command that downloads a file from a remote URL.

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + path, callback);
}

The shell command will, however, fail to work as intended if the input contains spaces or other special characters interpreted in a special way by the shell. Even worse, a client might pass in user-controlled data, not knowing that the input is interpreted as a shell command. This could allow a malicious user to provide the input http://redactedaws.org; cat /etc/passwd in order to execute the command cat /etc/passwd. To avoid such potentially catastrophic behaviors, provide the inputs from exported functions as an argument that does not get interpreted by a shell:

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.execFile("wget", [path], callback);
}

As another example, consider the following code which is similar to the preceding, but pipes the output of wget into wc -l to count the number of lines in the downloaded file.

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + path + " | wc -l", callback);
};

In this case, using child_process.execFile is not an option because the shell is needed to interpret the pipe operator. Instead, you can use shell-quote to escape the input before embedding it in the command:

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + shellQuote.quote([path]) + " | wc -l", callback);
};

References

Command Injection
shell-quote
CWE-78
CWE-88

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Pull request labels are added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@odaysec odaysec requested a review from a team as a code owner April 12, 2025 12:28
@sobolk sobolk merged commit c55512f into aws-amplify:dev Apr 21, 2025
5 of 6 checks passed
@sobolk
Copy link
Copy Markdown
Contributor

sobolk commented Apr 21, 2025

@odaysec Thank you for the contribution.

Please keep in mind that lambdaExecutableDir input to execa is prepared by the CLI using Amplify project's path. Therefore chances of successful exploitation are none.
Also, the "amplify project path" -> CLI -> execa data flow happens on single developer machine.

The lambdaExecutablePath is constructed here

const lambdaExecutableDir = path.join(request.srcRoot, BIN_LOCAL);
const lambdaExecutablePath = path.join(lambdaExecutableDir, MAIN_BINARY);
from request.srcRoot.

The request.srcRoot is created here

const resourcePath = path.join(pathManager.getBackendDirPath(), categoryName, resourceName);
const { pluginId, functionRuntime }: FunctionBreadcrumbs = context.amplify.readBreadcrumbs(categoryName, resourceName);
const runtimeManager: FunctionRuntimeLifecycleManager = await context.amplify.loadRuntimePlugin(context, pluginId);
return ({ event }) =>
runtimeManager.invoke({
handler,
event: JSON.stringify(event),
runtime: functionRuntime,
srcRoot: resourcePath,
envVars,
});
.

Amplify CLI controls all inputs involved in lambdaExecutablePath computation. resourceName is validated against the project before invoking the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants